test(reborn): E-TRIGGERED-SUBMIT enabler — triggered-turn submit seam - #5516
Conversation
Add submit_triggered_turn() on RebornIntegrationHarness: builds a synthetic TriggerFire + TriggerMaterializedPrompt::for_fire and hands them to the REAL production trusted_trigger_fire_submitter (over a fresh InMemoryConversationServices dedicated to the trigger path, mirroring production's own local-dev wiring in build_trigger_poller_services_from_conversation_services), so the submitted turn carries a genuine TurnOriginKind::ScheduledTrigger origin end to end. Driving test (reborn_integration_triggered_submit.rs) submits one triggered turn and asserts product_context.origin == ScheduledTrigger via TurnCoordinator::get_run_state at the coordinator boundary. The submitted run then executes autonomously on the harness's background scheduler and fails benignly on a model-gateway scope-miss (no scripted gateway registered for the trigger's own resolved scope) — harmless, since product_context is persisted synchronously at submit and untouched by that later failure. Driving a triggered run to model completion is C-TRIGGERED-DELIVERY, not this seam. Mutation-verified: flipped the TrustedInboundKind::Trigger classification arm in ironclaw_conversations::inbound (Trigger -> TrustedOther), confirmed the test goes RED (Some(Inbound) != Some(ScheduledTrigger)), reverted to a 0-line diff, confirmed GREEN. Unlocks C-TRIGGERED-ORIGIN / C-TRIGGERED-DELIVERY (separate PRs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a test-only triggered-submit harness seam, wires it into reborn support, updates dev-dependencies for test support, and verifies persisted run state carries ChangesTriggered submit harness and test
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Test as RebornIntegrationHarness caller
participant Harness as RebornIntegrationHarness
participant Conversations as InMemoryConversationServices
participant Submitter as trusted_trigger_fire_submitter
participant Coordinator as coordinator
Test->>Harness: submit_triggered_turn(prompt)
Harness->>Conversations: pre-pair canonical trusted external actor
Harness->>Submitter: TrustedTriggerSubmitRequest from TriggerFire
Submitter->>Coordinator: submit triggered turn
Submitter-->>Harness: Accepted(run_id, turn_scope) or Replayed
Harness-->>Test: TriggeredSubmission{run_id, turn_scope}
Test->>Coordinator: get_run_state(turn_scope, run_id)
Coordinator-->>Test: run_state with product_context.origin
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces integration testing support for the trusted-trigger submission seam (E-TRIGGERED-SUBMIT), adding the necessary dependencies and test files to verify that the TurnOriginKind::ScheduledTrigger origin propagates correctly to the persisted run state. The review feedback suggests minor performance optimizations by removing unnecessary clones of fire and submission.turn_scope where ownership can be directly transferred.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
Adds an integration-test harness seam to submit a triggered turn through the real production trusted-trigger submission path, so the resulting run is persisted with TurnOriginKind::ScheduledTrigger and can be asserted at the TurnCoordinator boundary.
Changes:
- Introduces
RebornIntegrationHarness::submit_triggered_turn()(E-TRIGGERED-SUBMIT seam) that constructs a syntheticTriggerFire+ materialized prompt and submits it viaironclaw_conversations::trusted_trigger_fire_submitter. - Adds a driving integration test asserting the persisted run state carries
TurnOriginKind::ScheduledTrigger. - Enables the needed test constructor by turning on
ironclaw_triggers’test-supportfeature for dev-dependencies and addsironclaw_conversationsas a dev-dependency.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/support/reborn/triggered_submit.rs | New harness seam that submits a triggered turn through the real trusted-trigger submitter and returns { run_id, turn_scope }. |
| tests/support/reborn/mod.rs | Exposes the new triggered_submit support module. |
| tests/reborn_integration_triggered_submit.rs | New driving test that asserts ScheduledTrigger origin is present in TurnCoordinator::get_run_state(). |
| Cargo.toml | Adds dev-deps/feature flags needed for the new seam (ironclaw_triggers with test-support, plus ironclaw_conversations). |
| Cargo.lock | Locks the new dev-dependency graph including ironclaw_conversations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reborn integration-tier coverageLine coverage (Reborn crates): 14.61% — 9205 / 63013 lines Per-crate breakdown (12 crates, lowest-covered first)
This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout. |
|
🚅 Deployed to the ironclaw-pr-5516 environment in ironclaw-ci-preview
|
- Avoid unnecessary fire.clone() by materializing the prompt (which only borrows fire) before moving fire into new_for_test. - Avoid unnecessary submission.turn_scope.clone() in the driving test by moving it instead (submission is not used afterward). - Use try_pair_external_actor (not the infallible pair_external_actor wrapper) so a pre-pairing failure surfaces at the seam boundary instead of resurfacing later as an indirect binding-resolution error. Flagged independently by gemini-code-assist and Copilot PR review bots on #5516. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add a Reborn test harness seam that submits triggered turns through the production trusted trigger submitter and verifies scheduled-trigger origin persistence.
Stats: 2 findings (from 2 raw, 2 after dedup) across 1 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0
Findings
- Medium Cross-layer guarantee is documented but not enforced (
tests/support/reborn/triggered_submit.rs:77-82, confidence 75) — anchor: AGENTS.md:84
AGENTS.md requires comments that promise guarantees across layers to be enforced by code/tests or softened. This comment promises the later scheduler scope-miss leaves product_context untouched, but the new test only reads product_context immediately after submit and never waits for or re-reads after that later failure. - Nit Comment bakes transient PR contention into committed source (
tests/support/reborn/triggered_submit.rs:38-43, confidence 50) — anchor: tests/support/reborn/triggered_submit.rs:38-43
The HarnessResult comment explains the local alias pattern, but it also records that builder.rs is under file contention with other in-flight PRs. That temporary review context will go stale after those PRs land and makes the source harder to trust as durable documentation.
- Drop the transient "builder.rs is under file-contention with other in-flight PRs" clause from the HarnessResult comment — durable rationale (sibling modules duplicate the private alias) stands on its own without a note that goes stale once those PRs land. - Soften the submit_triggered_turn doc comment: it previously claimed product_context "is untouched by [the background scheduler's later scope-miss] failure, so it is observable regardless" — a cross-layer guarantee the driving test does not actually enforce (it only reads product_context immediately after submit, never after the later failure). Per AGENTS.md, an unenforced cross-layer guarantee must be enforced or softened; extending the test to wait for and assert on the background failure was considered and rejected (ran through /thermo-nuclear-code-quality-review) — it would blur this enabler PR's deliberately minimal smoke-slice scope into C-TRIGGERED- DELIVERY's territory (background-scheduler failure/retry behavior) and couple the test to scheduler retry timing for a claim already verified true by static analysis across two prior review passes. Softened to state the comment's actual, tested scope instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every existing TriggerInboundContentRef/InboundMessageContentRef call site in the codebase (ironclaw_triggers, ironclaw_conversations) uses a "content:" prefix. The prefix isn't parsed by anything (validate_inbound_content_ref only checks length/charset), so this is a pure consistency fix flagged by PR review, not a functional change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves conflicts with PR-E2 (#5514, E-SKILL/E-DURABLE/E-GATEWAY seam constructors) and E-TRIGGERED-SUBMIT (#5516), both landed on main after this branch was cut and both touching the same shared Reborn integration- test harness scaffolding (tests/support/reborn/{builder,group,harness, mod,assertions}.rs). Only builder.rs had textual conflict markers (kept both with_safety_context() and park_model()); the other four files auto-merged via 3-way merge and were manually verified to retain both sides' additions: - ours: safety_context (C-SAFETY), push_response_body/web_access_tools/ capability_backend.rs extraction/assert_egress_body_contains_any (C-WEBACCESS) - theirs: park_model, skill_activation_tools, triggered_submit module, E-SKILL/E-GATEWAY/E-DURABLE plumbing Two real compile breaks surfaced only by `cargo test --no-run` (workspace `cargo check` doesn't build test targets) and fixed: - builder.rs referenced GroupCapability (from main's E-DURABLE test) without it being imported - web_access_tools() was missing the skill_activation_source field main added to HostRuntimeCapabilityHarness Also fixed 4 stale doc-comment references to WebAccessTestHandler/ web_access_test_error, which were deleted by an earlier commit on this branch (production register_bundled_web_access_first_party_handlers is used directly instead) but the doc comments describing web_access_tools()/ WebAccessTools/the error-mapping test/WEB_ACCESS_PROVIDER_ID weren't all updated at the time — two were already flagged by PR review (Copilot, CodeRabbit), two more found during this merge. Verified: cargo check --workspace --all-features clean; all 9 relevant reborn_integration_* test binaries pass (web_access, safety, greeting, http_matcher, secret_injection, cancel, durable, skill_activate, triggered_submit — 218 total tests, 0 failed); cargo clippy -p ironclaw --tests --all-features zero warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Tier-1 enabler seam from the Reborn backend coverage roadmap:
submit_triggered_turn()onRebornIntegrationHarnesssubmits a turn through the real productionTrustedTriggerFireSubmitter(not a hand-forged request), so the turn carries a genuineTurnOriginKind::ScheduledTriggerorigin end to end. Unlocks C-TRIGGERED-ORIGIN / C-TRIGGERED-DELIVERY (separate later PRs).Seam shape
TriggerFire+TriggerMaterializedPrompt::for_fire(test ctor) and hands them to the productionironclaw_conversations::trusted_trigger_fire_submitter.InMemoryConversationServicesdedicated to the trigger path — the exact conversation-services type production's own local-dev build wires for this purpose (build_trigger_poller_services_from_conversation_servicesinironclaw_reborn_composition::runtime.rs), not the harness's unrelated direct-chat product-workflow binding service (a distinct axis even in real production).pair_external_actor) before submitting, using the sameTRIGGER_TRUSTED_*constants production's ownTriggerTrustedInboundBinding::for_firederives from — required becauseresolve_actorhard-failsBindingRequiredwithout it.Arc<dyn TurnCoordinator>through unchanged, so the submitted run lands in the same turn store/scheduler as every other harness turn.TriggeredSubmission { run_id, turn_scope }— the trigger's own resolved scope, read back from the submitter'sAcceptedoutcome (the trusted-submit contract forbids re-deriving binding keys from aTriggerFire, so the seam consumes rather than reconstructs it).Driving test
tests/reborn_integration_triggered_submit.rssubmits one triggered turn and assertsTurnCoordinator::get_run_state(...).product_context.origin == TurnOriginKind::ScheduledTriggerat the coordinator boundary — a smoke slice proving the seam is live, not the exhaustive origin/delivery matrix (left to the C-PRs).The submitted run then executes autonomously on the harness's background
TurnRunScheduler; no scripted model gateway is registered for the trigger's own resolved scope, so it fails benignly on a scope-miss (ScopeRegistryGateway'sConfigurationErrorsentinel). This is harmless:product_context(carrying the origin) is persisted synchronously at submit time and untouched by that later failure. Driving a triggered run to model completion is C-TRIGGERED-DELIVERY's job, not this seam's.Mutation-verify evidence
Flipped the
TrustedInboundKind::Triggerclassification arm inironclaw_conversations::inbound(TrustedTrigger→TrustedOther), rebuilt, and confirmed the driving test goes RED for the right reason:Reverted — confirmed
git diffon that file is a 0-line diff — and confirmed GREEN again.Verification
cargo clippy --all --benches --tests --examples --all-features: zero warnings introduced (21 pre-existing, unrelatedcriterion::black_boxdeprecation warnings inironclaw_safetybenches, not touched by this diff).cargo test --features libsql --test reborn_integration_triggered_submit:test result: ok. 22 passed; 0 failed; 1 ignored— confirmed the new test actually ran (not just compiled).FilesystemConversationBindingService/FilesystemSessionThreadServicetotrusted_trigger_fire_submitter, which actually bounds on distinct same-namedironclaw_conversations-local traits) plus a runtime-blocking missingpair_external_actorpre-seed — both fixed before implementation, confirmed by a third focused verification pass, then implemented and post-impl reviewed clean.Scope
Only touches
Cargo.toml/Cargo.lock(two new/modified dev-dependencies),tests/support/reborn/mod.rs(one newpub modline), and two new files. Does not touchgroup.rs/builder.rs— no contention with the in-flight PR-E2 (#5514) or PR-C1.Test plan
cargo clippy --all --benches --tests --examples --all-features— zero new warningscargo test --features libsql --test reborn_integration_triggered_submit— 1 new test passes, confirmed it ran